Skip to content

Conversation

@pvsr
Copy link

@pvsr pvsr commented Jan 22, 2025

Normally when the request body is empty, handle_body is only called for POST and PUT requests, since other methods shouldn't have Content-Type and Content-Length headers for empty bodies. But one case is missed in the empty body detection, an iolist containing empty binaries. The gleam hackney wrapper represents empty bodies as [<<>>], triggering this bug and causing gleam-lang/hackney#5.

This PR expands the empty body detection to check if the result of iolist_to_binary is empty.

@pvsr pvsr marked this pull request as draft January 22, 2025 05:24
@pvsr pvsr force-pushed the empty-req-body-detection branch 2 times, most recently from 37cef62 to 87f0d7e Compare January 22, 2025 05:29
S = hackney_headers_new:get_value(<<"content-length">>, Headers),
{S, CT, Body0};

_ when is_list(Body0) -> % iolist case
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case is now handled before calling handle_body.

@pvsr pvsr marked this pull request as ready for review January 22, 2025 05:31
@benoitc benoitc self-assigned this Jan 22, 2025
@pvsr pvsr force-pushed the empty-req-body-detection branch 2 times, most recently from 1e85121 to 6e5534c Compare October 27, 2025 05:29
@pvsr
Copy link
Author

pvsr commented Oct 27, 2025

I rebased and added some integration tests.

@pvsr pvsr force-pushed the empty-req-body-detection branch from 6e5534c to 3580e75 Compare October 27, 2025 05:32
{Headers2, ReqType0, Body0, Client0};
[] ->
{Headers2, ReqType0, Body0, Client0};
_ when is_list(Body0) ->
Copy link
Owner

@benoitc benoitc Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we don't anything specif for iolist why not doing it at first level and then check for empty or not body ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! I was trying to keep my changes isolated but that's a better way of doing it. Just pushed a new version.

@pvsr pvsr force-pushed the empty-req-body-detection branch 3 times, most recently from 72a51a9 to 011f3e8 Compare October 27, 2025 21:52
@pvsr pvsr requested a review from benoitc October 27, 2025 21:53
@pvsr pvsr force-pushed the empty-req-body-detection branch from 011f3e8 to 1a118bf Compare October 27, 2025 22:00
Fixes non-POST/PUT requests with a body of [<<>>] receiving
Content-Type and Content-Length headers despite not having a body.
@pvsr pvsr force-pushed the empty-req-body-detection branch from 1a118bf to 6ff6aba Compare October 27, 2025 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants